Skip to content

Conversation

@novemberborn
Copy link
Member

I was going to document watch mode but then realized I hadn't really read the README in a while. I figured I'd use my fresh set of eyes to try and improve it instead.

I've tried to document rationale for each change in the individual commits. I'll squash these into a single commit before merging. Happy to wait until some of the outstanding PRs are in to avoid those having to be rebased again.

Hopefully this version of the readme has some better examples and brings various nuances and hidden behaviors to the fore. Let me know what else I should change, or revert 😄

P.S. Apologies to the awesome translators for this massive update. You folks rock.

@jamestalmage
Copy link
Contributor

These should remain tab indented.

I just pushed up the fix-readme-improvements branch which removes the whitespace changes. I was trying to come up with a way to script that (I was unsuccessful).

@sindresorhus
Copy link
Member

Just some minor inline nits, but generally looks very good :)

@jamestalmage
Copy link
Contributor

It is probably a good idea to leave this branch accessible with no squashing of commits. It may be easier for translators to follow along commit by commit. We should still only push a single squashed commit to master, but lets leave the history here to help the translators.

@forresst
Copy link
Contributor

forresst commented Mar 4, 2016

@jamestalmage

It is probably a good idea to leave this branch accessible with no squashing of commits. It may be easier for translators to follow along commit by commit. We should still only push a single squashed commit to master, but lets leave the history here to help the translators.

👍
It's a very good idea

readme.md Outdated

`.only` applies across all test files, so if you use it in one file, no tests from the other file will run.

### Skip-tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to retitle to Skipping tests to be consistent with the other titles.

@novemberborn
Copy link
Member Author

These should remain tab indented.

Fair enough. I'll make them consistent in the other direction. Keeping spaces for JSON blocks and CLI output.

I was trying to come up with a way to script that (I was unsuccessful).

@jamestalmage script what? I used the Whitespace package in Atom which did the trick, although the right indentation varies by code block which is annoying.

It is probably a good idea to leave this branch accessible with no squashing of commits. It may be easier for translators to follow along commit by commit. We should still only push a single squashed commit to master, but lets leave the history here to help the translators.

Yes I was planning to squash and merge directly on master from my own machine and leaving this PR as is.


Updated. Added a bunch more changes (see commits after reword faq).

Timing wise perhaps best to land #592 and #573 first. It's easier to update this PR so those new doc inserts read well with the rest of the changes here than it is to update them in their respective PRs.

@sindresorhus
Copy link
Member

although the right indentation varies by code block which is annoying.

Blame npm. They're the ones requiring 2 space indent.

Timing wise perhaps best to land #592 and #573 first.

👍

@jamestalmage
Copy link
Contributor

script what?

A way to apply a series of commits, excluding whitespace only changes. git flags like --ignore-spacing-changes had me thinking it was doable. Turns out those flags don't do what I assumed they did

@novemberborn
Copy link
Member Author

Updated with improved --match documentation. @kasperlewau does it look OK to you (see last commit).

@novemberborn novemberborn mentioned this pull request Mar 6, 2016
@kasperlewau
Copy link
Contributor

Had a read through and I think that looks just fine @novemberborn 👍

@vadimdemedes
Copy link
Contributor

If @novemberborn made all requested changes, is it good to go or anyone else has some more suggestions?

@novemberborn
Copy link
Member Author

@vdemedes I'm waiting for #573 to land first.

@sindresorhus
Copy link
Member

@novemberborn #573 was landed. Can you fix the merge conflict and squash merge this to master?

Use fenced code blocks, if anybody got confused by the $ character in the inline
code samples maybe that's clearer.

Mention an approach that does not use ava --init.
The documentation uses 'directory' not 'folder'.
* Explain what directory recursion means.
* Mention that node_modules directories are ignored.
* Emphasize that fixtures/helpers/node_modules are always ignored.
* Reword how the _ prefix is useful (the reason seemed to apply to the previous
sentence as a whole).
Clarify that tests run concurrently and can be both synchronous and
asynchronous, with synchronous being the default.
* Consistently use 'callback' for the function that's passed to the test methods.
* Use "test title" not "test name" and improve title explanation.
* Reword "context" to be the "execution object" to avoid confusion with the
context that beforeEach/afterEach hooks can manipulate.
* Explain the convention to name this execution object 't'.
* Be clearer which non-assertion methods are available on the execution object.
* Reword the explanation
* Link to tap and tape
* More examples
* Link back to the explanatory section in the .plan(count) docs
* Remind users that AVA runs tests concurrently.
* Emphasize that serial tests are executed before concurrent ones.
* Note that test files are still run concurrently unless the --serial flag is used.
* Prioritize that this should be used during development.
* Explicitly mention the modifier, not just in the example.
* Explain that this applies across all test files.
* Hopefully clarify the various nuances in when these hooks execute
* Note that before/after hooks may not be necessary due to process isolation
* Clarify that context sharing is not available to before/after hooks
* Be more specific about what modifiers are
* Simplify example
* Reword bit about temporarily adding .skip or .only
* Prefer 'library' over 'module'
* Stipulate that the library must throw an exception
* Link to the built-in assertions and assertion planning sections
* Be less apologetic
* Recommend the require hook
* Suggest transpiling in a separate process
* Indent the 'skipping assertions' and 'enhanced' sections
* Change title for the 'enhanced' section
* Remove simple example, contrast AVA with Node's default assert library
* Add to TOC
* Reword Temp files section
* Reword Cod ecoverage section, drop mention of .babelrc files
* tap rather than node-tap
* less personal review of other test runners
* reword custom reporter section
* ensure the headings are questions, and the answers are phrased like answers
* https link for stack overflow
* Title in line with other titles
* Reword description
* Title in line with other titles
* Reword description
Clarify that .plan() and .end() are on the 't' object.
* Consistent title
* Reword description
* Improve examples
These sections read better when combined. I've also reworded the config
documentation.

Changed Babel links to use HTTPS.
@novemberborn
Copy link
Member Author

Updated. I decided to combine the ES2015 and Babel config sections, it read a lot better that way. Also reworded it a little.

@spudly are you happy with my changes? See the last commit.

I'll merge if people are 👍

@sindresorhus
Copy link
Member

@novemberborn Looks good. Just merge now. We can follow up later with improvements if anyone has comments. This changes a lot of things, so would be good to get it merged asap.

@novemberborn novemberborn deleted the readme-improvements branch March 7, 2016 17:21
@sindresorhus
Copy link
Member

Thanks for leveling up the docs @novemberborn :)

@spudly
Copy link
Contributor

spudly commented Mar 7, 2016

@novemberborn, no complaints here. Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants